Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: added link to YT video to TSC page #1141

Merged
merged 10 commits into from
Dec 6, 2022
Merged

Conversation

Anurag607
Copy link
Contributor

Description

  • Added YT video link that explains how to join TSC to TSC page
  • The Link is added in the Question Card.

Related issue(s)

Resolves #809

Before :

image

After :

image

@netlify
Copy link

netlify bot commented Dec 5, 2022

Deploy Preview for asyncapi-website ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 8820aba
🔍 Latest deploy log https://app.netlify.com/sites/asyncapi-website/deploys/638f295ecccd0100091aa0a4
😎 Deploy Preview https://deploy-preview-1141--asyncapi-website.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

@Anurag607 Anurag607 changed the title feat(enhancement) : added link to YT video to TSC page feat : added link to YT video to TSC page Dec 5, 2022
@github-actions
Copy link

github-actions bot commented Dec 5, 2022

⚡️ Lighthouse report for the changes in this PR:

Category Score
🔴 Performance 49
🟠 Accessibility 88
🟠 Best practices 83
🟢 SEO 100
🔴 PWA 30

Lighthouse ran on https://deploy-preview-1141--asyncapi-website.netlify.app/

@Anurag607 Anurag607 changed the title feat : added link to YT video to TSC page feat: added link to YT video to TSC page Dec 5, 2022
Copy link
Member

@akshatnema akshatnema left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Want to have some discussion here regarding the place it is to be added.

image

@derberg @Anurag607 I find this place to be more appropriate to add the Youtube tutorial link for TSC members, as it describes the notion of the video as well as it is at the top of the page. WDYT?

<div className="my-4">
Want to become a member?
Follow this
<Link href='https://www.youtube.com/watch?v=uG_aLF9Z1F0'>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, use TextLink component https://github.com/asyncapi/website/blob/master/components/typography/TextLink.js, along with target="blank" to open the video in new tab.

Copy link
Member

derberg commented Dec 5, 2022

actually both locations are good. We can have it in both

@Anurag607
Copy link
Contributor Author

@derberg @akshatnema, so should I add it in the Question Card or at the end of the paragraph?
Also I think if the link were to be added in the paragraph, then the card will not be needed

@akshatnema
Copy link
Member

@derberg @akshatnema, so should I add it in the Question Card or at the end of the paragraph?
Also I think if the link were to be added in the paragraph, then the card will not be needed

It's better you do both implementations.

@Anurag607
Copy link
Contributor Author

I've added links to both the TSC para and the question Card and used TextLink instead of Link
image

@@ -72,6 +73,11 @@ export default function TSC() {
can also build a great AsyncAPI-based project that we don't have
yet in our GitHub organization and donate it (we'll ask you to
stay as a maintainer).
Follow this
<TextLink href="https://www.youtube.com/watch?v=uG_aLF9Z1F0" target="_blank" className="text-base text-blue-500 hover:text-sky-400 no-underline">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<TextLink href="https://www.youtube.com/watch?v=uG_aLF9Z1F0" target="_blank" className="text-base text-blue-500 hover:text-sky-400 no-underline">
<TextLink href="https://www.youtube.com/watch?v=uG_aLF9Z1F0" target="_blank" className="text-base font-normal text-blue-500 hover:text-sky-400 no-underline">

The font-weight of the text is getting large, so it should be made according to the remaining text in the paragraph.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done..

<div className="my-4">
Want to become a member?
Follow this
<TextLink href="https://www.youtube.com/watch?v=uG_aLF9Z1F0" target="_blank" className="text-sky-600 hover:text-sky-400 no-underline">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<TextLink href="https://www.youtube.com/watch?v=uG_aLF9Z1F0" target="_blank" className="text-sky-600 hover:text-sky-400 no-underline">
<TextLink href="https://www.youtube.com/watch?v=uG_aLF9Z1F0" target="_blank" className="font-normal text-sky-600 hover:text-sky-400 no-underline">

same here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done...

@derberg
Copy link
Member

derberg commented Dec 5, 2022

instead of old YT recording we finally have an official document you can link to -> https://github.com/asyncapi/community/blob/master/TSC_MEMBERSHIP.md

@Anurag607
Copy link
Contributor Author

@derberg I will then replace the YT link with the docs link

@akshatnema
Copy link
Member

@Anurag607 I see that the above CSS attributes are still not applied even after adding classes. Kindly add twMerge function in the className of the TextLink component to modify the component according to the changes we need. You can take reference from here - https://github.com/asyncapi/website/blob/master/components/buttons/Button.js#L17

@Anurag607
Copy link
Contributor Author

@akshatnema I have updated the PR according to the review

@@ -72,6 +74,11 @@ export default function TSC() {
can also build a great AsyncAPI-based project that we don't have
yet in our GitHub organization and donate it (we'll ask you to
stay as a maintainer).
Follow this
<TextLink href="https://github.com/asyncapi/community/blob/master/TSC_MEMBERSHIP.md" target="_blank" className={twMerge(`text-base font-normal text-blue-500 hover:text-sky-400 no-underline`)}>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the twMerge function from file as it is not needed here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have updated the PR according to the review

Copy link
Member

@akshatnema akshatnema left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM @Anurag607, anything from your side @derberg?

Copy link
Member

derberg commented Dec 6, 2022

I'm good, thanks so much @Anurag607 !

@akshatnema
Copy link
Member

/rtm

@asyncapi-bot asyncapi-bot merged commit 32e0178 into asyncapi:master Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add to TSC page link to YT video that explains how to join TSC
4 participants